Skip to content

crater experiment: de-const-stabilize UnsafeCell::get and various as_ptr methods #129399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

crater experiment: de-const-stabilize UnsafeCell::get and various as_ptr methods #129399

wants to merge 1 commit into from

Conversation

RalfJung
Copy link
Member

These should not have been stabilized before const_refs_to_cell.
For now I mostly want to see whether any code out there actually uses these methods, since it shouldn't be possible to do anything useful with them...
Cc @rust-lang/wg-const-eval

@rustbot
Copy link
Collaborator

rustbot commented Aug 22, 2024

r? @Noratrieb

rustbot has assigned @Noratrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 22, 2024
@RalfJung
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
@bors
Auto merge of rust-lang#129399 - RalfJung:no-const-refs-to-cell, r=<try>
2331520 
crater experiment: de-const-stabilize UnsafeCell::get and various as_ptr methods

These should not have been stabilized before const_refs_to_cell.
For now I mostly want to see whether any code out there actually uses these methods, since it shouldn't be possible to do anything useful with them...
Cc `@rust-lang/wg-const-eval`
@bors
Copy link
Collaborator

bors commented Aug 22, 2024

⌛ Trying commit c187c98 with merge 2331520...

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
#16 2.797 Building wheels for collected packages: reuse
#16 2.798   Building wheel for reuse (pyproject.toml): started
#16 3.046   Building wheel for reuse (pyproject.toml): finished with status 'done'
#16 3.047   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#16 3.047   Stored in directory: /tmp/pip-ephem-wheel-cache-871w9awg/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#16 3.050 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#16 3.444 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#16 3.444 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#16 DONE 3.5s
---
    Checking winapi-util v0.1.9
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
  --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:71:22
   |
71 |             unsafe { (*(self as *const Self as *const UnsafeCell<*mut T>)).get() }
   |
   = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
---

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
---

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
411 |   atomic_int!(AtomicI8, i8);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
413 |   atomic_int!(AtomicU8, u8);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
415 |   atomic_int!(AtomicI16, i16);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

    Checking pad v0.1.6
    Checking pad v0.1.6
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
417 |   atomic_int!(AtomicU16, u16);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
420 |   atomic_int!(AtomicI32, i32);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
423 |   atomic_int!(AtomicU32, u32);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
432 |   atomic_int!(AtomicI64, i64);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:198:25
    |
122 | / macro_rules! atomic_int {
123 | |     ($atomic_type:ident, $int_type:ident) => {
124 | |         #[repr(transparent)]
125 | |         pub(crate) struct $atomic_type {
...   |
198 | |                         (*(self as *const Self as *const UnsafeCell<$int_type>)).get()
...   |
405 | |     };
406 | | }
    | |_- in this expansion of `atomic_int!`
    | |_- in this expansion of `atomic_int!`
...
441 |   atomic_int!(AtomicU64, u64);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/fallback/mod.rs:369:17
    |
106 | / macro_rules! atomic {
107 | |     ($atomic_type:ident, $int_type:ident, $align:literal) => {
108 | |         #[repr(C, align($align))]
109 | |         pub(crate) struct $atomic_type {
369 | |                 self.v.get()
    | |                 ^^^^^^^^^^^^
...   |
372 | |     };
372 | |     };
373 | | }
    | |_- in this expansion of `atomic!`
...
385 |   atomic!(AtomicI128, i128, 16);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/fallback/mod.rs:369:17
    |
106 | / macro_rules! atomic {
107 | |     ($atomic_type:ident, $int_type:ident, $align:literal) => {
108 | |         #[repr(C, align($align))]
109 | |         pub(crate) struct $atomic_type {
369 | |                 self.v.get()
    | |                 ^^^^^^^^^^^^
...   |
372 | |     };
372 | |     };
373 | | }
    | |_- in this expansion of `atomic!`
...
386 |   atomic!(AtomicU128, u128, 16);
    |
    = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

error: `UnsafeCell::<T>::get` is not yet stable as a const fn

@tgross35
Copy link
Contributor 

    Checking winapi-util v0.1.9
error: `UnsafeCell::<T>::get` is not yet stable as a const fn
  --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/portable-atomic-1.7.0/src/imp/core_atomic.rs:71:22
   |
71 |             unsafe { (*(self as *const Self as *const UnsafeCell<*mut T>)).get() }
   |
   = help: add `#![feature(const_unsafecell_get)]` to the crate attributes to enable

Well, there's your crater :)

regressed: root results (1 different result, 1003546 regressed crates in total)

@RalfJung
Copy link
Member Author

RalfJung commented Aug 22, 2024 •
edited
Loading

Yeah that's the portable-atomic crate itself also providing a const fn as_ptr(&self) -> *mut _ on an interior mutable type, similar to our own Atomic* types.

I hope there's no way to actually call that function because self has interior mutability. But we'll have a hard time testing this hypothesis because portable-atomic will fail to build. Is there some way we can substitute the crate sources of portable-atomic for this crater run...? (to make that function not be const fn)

@tgross35
Copy link
Contributor

I have done some ugly but functional use of [patch.crates-io] to use my own fork of compiler_builtins, e.g. #127561 (comment). Maybe that works here?

@bors
Copy link
Collaborator

bors commented Aug 22, 2024

☀️ Try build successful - checks-actions
Build commit: 2331520 (2331520994fdd8f5407d0fe95377391fc96ccf2c)

@oli-obk
Copy link
Contributor

oli-obk commented Aug 22, 2024

Alternatively you can use const_eval_select together with todo!() on the const side. Not the same thing, but a check whether it's actually used constly

@RalfJung
Copy link
Member Author

Turns out there is a way to call this:

use std::cell::UnsafeCell;

const FOO2: () = {
    let mut x = 42;
    let y: &UnsafeCell<i32> = unsafe { std::mem::transmute(&x) };
    y.get();
};

Writing to this is obviously UB, though.^^

It'd be fun to figure out whether this is actually done anywhere, but I don't see a reasonably easy way to do that so 🤷

@RalfJung RalfJung closed this Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Noratrieb Noratrieb

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@RalfJung @rustbot @bors @rust-log-analyzer @tgross35 @oli-obk @Noratrieb